Fix transport session id mismatch with sessionId #176
Conversation
Signed-off-by: JermaineHua <crazyhzm@apache.org>
|
good job，I also found this problem, now get the correct seesion, not inside the mcp session, but inside the transport～ |
There was a problem hiding this comment.
Thank you for the proposal! I agree we have a mismatch and we need a way to provide the session id to both the transport and the session objects. I would propose that the McpServerSession.Factory expose a new method, generateId which can be fed into both of these objects. Would you like to apply such a change?
Yes, I'll make some revisions to this part. |
Signed-off-by: JermaineHua <crazyhzm@apache.org>
# Conflicts: # mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java
Signed-off-by: JermaineHua <crazyhzm@apache.org>
There was a problem hiding this comment.
Thanks for the work on this. It is almost ready, a few nitpicks about imports.
As a next step we should unify the way the server transports log and include the session id everywhere. But please don't create a PR for it just yet.
@tzolov in 0.11.0 release notes we need to highlight that McpServerSession.Factory went through a breaking change impacting custom transport implementors, not end users per se.
| @@ -2,6 +2,7 @@ | |||
|
|
|||
| import java.io.IOException; | |||
| import java.util.Map; | |||
| import java.util.UUID; | |||
| @@ -247,7 +247,7 @@ private ServerResponse handleSseConnection(ServerRequest request) { | |||
| return ServerResponse.status(HttpStatus.SERVICE_UNAVAILABLE).body("Server is shutting down"); | |||
| } | |||
|
|
|||
| String sessionId = UUID.randomUUID().toString(); | |||
| @@ -208,7 +208,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) | |||
| response.setHeader("Connection", "keep-alive"); | |||
| response.setHeader("Access-Control-Allow-Origin", "*"); | |||
|
|
|||
| String sessionId = UUID.randomUUID().toString(); | |||
| @@ -12,6 +12,7 @@ | |||
| import java.io.Reader; | |||
| import java.nio.charset.StandardCharsets; | |||
| import java.util.Map; | |||
| import java.util.UUID; | |||
| @@ -16,6 +16,7 @@ | |||
| package io.modelcontextprotocol; | |||
|
|
|||
| import java.util.Map; | |||
| import java.util.UUID; | |||
| @@ -10,6 +10,7 @@ | |||
| import java.io.PrintStream; | |||
| import java.nio.charset.StandardCharsets; | |||
| import java.util.Map; | |||
| import java.util.UUID; | |||
Motivation and Context
Fix #168
How Has This Been Tested?
Breaking Changes
Types of changes
Checklist
Additional context